-
Notifications
You must be signed in to change notification settings - Fork 4.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[RNMobile] Fix Issue with Search block stealing focus after media updated for image block #31393
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This fixes an issue on iOS where search block would steal the focus from the image block after selecting an image.
Now that the isSelected state of the search input field is no longer defaulted to true (because it was stealing focus from other components), we need to make sure that the block is actively selected prior to each test. While making these changes I also consolidated and reorganized the tests for code reusability.
This avoids some test flakiness on Android
AmandaRiu
added
[Block] Search
Affects the Search Block - used to display a search field
Mobile App - i.e. Android or iOS
Native mobile impl of the block editor. (Note: used in scripts, ping mobile folks to change)
labels
Apr 30, 2021
2 tasks
Size Change: 0 B Total Size: 1.31 MB ℹ️ View Unchanged
|
dcalhoun
approved these changes
May 3, 2021
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very excited to see the auto-focus issue resolved. It should be a solid improvement for users, particularly with screen readers. 👏🏻 💯
Tested on an iPhone SE and Samsung Galaxy S20. Unit and e2e tests run as well.
packages/react-native-editor/__device-tests__/gutenberg-editor-search.test.js
Show resolved
Hide resolved
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
[Block] Search
Affects the Search Block - used to display a search field
Mobile App - i.e. Android or iOS
Native mobile impl of the block editor. (Note: used in scripts, ping mobile folks to change)
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Mobile Gutenberg PR: wordpress-mobile/gutenberg-mobile#3442
Description
Fixes #31351 where the Search block was stealing the focus away from the Image block after the image was updated (iOS only). The fix itself was really straightforward - at some point I defaulted the
isPlaceholderSelected
state variable totrue
but this is not necessary and was forcing the search input element of the Search block to gain focus and take focus from the image block. Defaulting this tofalse
fixed the issue. While this change was tiny, it required fixes to both the Unit Test snapshot and the e2e tests.How has this been tested?
Tested the following scenario on both Android and iOS:
Run unit tests
Run e2e tests
IOS
Android
Screenshots
Before
Screen.Recording.2021-04-30.at.2.34.19.PM.mov
After
Screen.Recording.2021-04-30.at.2.35.02.PM.mov
Types of changes
Bug fix.
Checklist:
*.native.js
files for terms that need renaming or removal).